Skip to content

Fix multipart attachments with serialize_worker_startup#6071

Merged
nickva merged 1 commit into
mainfrom
fix-sws-with-multipart
Jul 22, 2026
Merged

Fix multipart attachments with serialize_worker_startup#6071
nickva merged 1 commit into
mainfrom
fix-sws-with-multipart

Conversation

@nickva

@nickva nickva commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Previously, if a multipart parser reached an eof before the declared length, it still let the writer wait and timeout (300s). This was especially noticed if users toggled serialize_worker_startup=true. Then, the first worker would buffer all the data (another undesirable behavior) until it reached a premature eof and then get stuck. Other workers wouldn't start either and the request would eventually crash.

Here we fix both issues:

  1. If we detect serialize_worker_startup=true then we switch back to the
    default parallel worker startup pattern. This how the MP parser was built to
    work. Otherwise it would buffer the whole attachment into memory until the
    first worker wasn't done and the others started. That defies the purpose of
    incremental attachment uploads. So to go with the grain of MP parser design we
    switch back to parallel worker startup.

  2. Let writers which wait on an EOF exit instead of deadlocking until a
    timeout. Writer waits for more bytes when the stream already reached EOF is
    because the user uploaded less than the declared number of bytes (attachment
    is too short). In that case we exit normally and let the parser monitors fire
    with "attachment shorter than expected" error.

Reproducer for the issue:

  • start dev/run cluster with 3 nodes
  • s:multicall(fun() -> config:set("fabric", "serialize_worker_startup","true") end)
  • curl -XPUT http://adm:[email protected]:15984/mptest

Before PR:

curl  --max-time 10 -XPUT 'http://adm:[email protected]:15984/mptest/short' -H 'Content-Type: multipart/related;boundary="abc123"' --data-binary $'--abc123\r\nContent-Type: application/json\r\n\r\n{"_attachments":{"ohai":{"follows":true,"content_type":"text/plain","length": 4}}}\r\n--abc123\r\n\r\noha\r\n--abc123--'
curl: (28) Operation timed out after 10005 milliseconds with 0 bytes received

After PR

curl  --max-time 10 -X PUT 'http://adm:[email protected]:15984/mptest/short' -H 'Content-Type: multipart/related;boundary="abc123"' --data-binary $'--abc123\r\nContent-Type: application/json\r\n\r\n{"_attachments":{"ohai":{"follows":true,"content_type":"text/plain","length": 4}}}\r\n--abc123\r\n\r\noha\r\n--abc123--'
{"error":"bad_request","reason":"attachment shorter than expected"}

Previously, if a multipart parser reached an eof before the declared length, it
still let the writer wait and timeout (300s). This was especially noticed if
users toggled `serialize_worker_startup=true`. Then, the first worker would
buffer all the data (another undesirable behavior) until it reached a premature
eof and then get stuck. Other workers wouldn't start either and the request
would eventually crash.

Here we fix both issues:

 1) If we detect serialize_worker_startup=true then we switch back to the
 default parallel worker startup pattern. This how the MP parser was built to
 work. Otherwise it would buffer the whole attachment into memory until the
 first worker wasn't done and the others started. That defies the purpose of
 incremental attachment uploads. So to go with the grain of MP parser design we
 switch back to parallel worker startup.

 2) Let writers which wait on an EOF exit instead of deadlocking until a
 timeout. Writer waits for more bytes when the stream already reached EOF is
 because the user uploaded less than the declared number of bytes (attachment
 is too short). In that case we exit normally and let the parser monitors fire
 with `"attachment shorter than expected"` error.

Reproducer for the issue:

 * start dev/run cluster with 3 nodes
 * `s:multicall(fun() -> config:set("fabric", "serialize_worker_startup","true") end)`
 * curl -XPUT 'http://adm:[email protected]:15984/mptest'

Before PR:

```
curl  --max-time 10 -XPUT 'http://adm:[email protected]:15984/mptest/short' -H 'Content-Type: multipart/related;boundary="abc123"' --data-binary $'--abc123\r\nContent-Type: application/json\r\n\r\n{"_attachments":{"ohai":{"follows":true,"content_type":"text/plain","length": 4}}}\r\n--abc123\r\n\r\noha\r\n--abc123--'
curl: (28) Operation timed out after 10005 milliseconds with 0 bytes received
```

After PR
```
curl  --max-time 10 -X PUT 'http://adm:[email protected]:15984/mptest/short' -H 'Content-Type: multipart/related;boundary="abc123"' --data-binary $'--abc123\r\nContent-Type: application/json\r\n\r\n{"_attachments":{"ohai":{"follows":true,"content_type":"text/plain","length": 4}}}\r\n--abc123\r\n\r\noha\r\n--abc123--'
{"error":"bad_request","reason":"attachment shorter than expected"}
```
@rnewson

rnewson commented Jul 22, 2026

Copy link
Copy Markdown
Member
diff --git i/src/fabric/src/fabric_doc_update.erl w/src/fabric/src/fabric_doc_update.erl
index 70081af73..cec747b67 100644
--- i/src/fabric/src/fabric_doc_update.erl
+++ w/src/fabric/src/fabric_doc_update.erl
@@ -103,8 +103,8 @@ handle_message(internal_server_error, Worker, #acc{} = Acc0) ->
     #acc{waiting_count = WC, grouped_docs = GrpDocs} = Acc0,
     NewGrpDocs = lists:keydelete(Worker, 1, GrpDocs),
     skip_message(start_workers(Acc0#acc{waiting_count = WC - 1, grouped_docs = NewGrpDocs}));
-handle_message(attachment_chunk_received, _Worker, #acc{} = Acc0) ->
-    {ok, Acc0};
+handle_message(attachment_chunk_received, Worker, #acc{} = Acc0) ->
+    {ok, start_workers_for_range(Worker#shard.range, Acc0)};
 handle_message({ok, Replies}, Worker, #acc{} = Acc0) ->
     #acc{
         waiting_count = WaitingCount,
@@ -470,6 +470,20 @@ start_remaining_workers(#acc{} = Acc) ->
         Acc#acc.grouped_docs
     ).

+start_workers_for_range(Range, #acc{} = Acc) ->
+    lists:foldl(
+        fun({Worker, Docs}, AccIn) ->
+            if
+                Worker#shard.range == Range ->
+                    start_worker(Worker, Docs, AccIn);
+                true ->
+                    AccIn
+            end
+        end,
+        Acc,
+        Acc#acc.grouped_docs
+    ).
+

@rnewson

rnewson commented Jul 22, 2026

Copy link
Copy Markdown
Member

we could instead start the other workers for that range on receipt of an attachment chunk. With this change I get the {"error":"bad_request","reason":"attachment shorter than expected"} error rather than a hang/timeout.

@nickva

nickva commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author
attachment_chunk_received

That could work but there may be more corner cases to worry about (do we always get an attachment_chunk_received, what if we have an empty body or empty parts) and it might be trickier to follow the logic. With pre-request firing check it's a bit more clear cut I think.

@nickva
nickva merged commit 84aa917 into main Jul 22, 2026
55 checks passed
@nickva
nickva deleted the fix-sws-with-multipart branch July 22, 2026 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants